Only enable BedrockSession packet logging in debug mode#6313
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates Geyser’s Bedrock session initialization to avoid enabling Cloudburst Bedrock packet logging unless Geyser is running in debug mode, preventing unnecessary per-packet log-record construction overhead (notably on BungeeCord due to its logging configuration).
Changes:
- Gate
BedrockServerSessionpacket logging behindconfig().debugMode()instead of always enabling it.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I understand that this is a simple fix and I appreciate the PR, but am I crazy to suggest that we should fix this in BungeeCord instead? This feels like one of those things that we could eventually remove as unneeded code, which is fair because the logger says that tracing is enabled. |
That's a fair point and was my first instinct too. I've been already in touch with the BungeeCord maintainers about this - fixing it on their end would be a breaking change, since plugins and downstream forks rely on the current Level.ALL behavior for their own logging. So it's not something that can realistically land there. Gating this behind |
With
setLogging(true), every inbound and outbound Bedrock packet triggersBedrockSession.logInbound/logOutbound. These calllog.trace(...)with the packet as a parameter, which materializespacket.toString()and routes the record through SLF4J.Normally an SLF4J TRACE call is cheap when the underlying logger filters it out - the record never gets constructed. But this breaks down on BungeeCord.
BungeeCord's
BungeeLoggersets its root logger toLevel.ALL. Every SLF4J call made from within BungeeCord's classloader (which includes Geyser when run as a plugin) is routed throughJDK14LoggerFactory.LOGGER, which points at that same Level.ALL logger. Because the logger accepts everything, the TRACE record is fully constructed -fillCallerData(),Throwable.getStackTrace(),MessageFormatterparameter expansion,packet.toString()- and only discarded later at the handler level.The result: on BungeeCord, every single Bedrock packet pays the full log-record construction cost, even though no log output is ever produced. The fix is to not ask for packet logging in the first place unless the user actually wants it.